Skip to content

Validate native geometry values on import - #9180

Open
HarukiMoriarty wants to merge 3 commits into
developfrom
nemo/geo-coordinate-validation
Open

Validate native geometry values on import#9180
HarukiMoriarty wants to merge 3 commits into
developfrom
nemo/geo-coordinate-validation

Conversation

@HarukiMoriarty

@HarukiMoriarty HarukiMoriarty commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • validate Point, list-based geometry, and Rect values once at the GeoArrow-to-native boundary
  • preserve GeoArrow empty-Point, slice, and null-parent semantics while rejecting malformed or non-finite X/Y values
  • validate WKB literals after decoding to native buffers without eagerly parsing stored WKB columns
  • benchmark the incremental cost with paired validation-disabled and validation-enabled import paths

Validation policy

  • Point: all-NaN ordinates represent empty; otherwise X/Y must be finite
  • List geometries: every reachable X/Y coordinate must be finite
  • Rect: X/Y bounds must be finite and ordered
  • Z/M remain outside the current 2-D validity policy
  • child values beneath null geometry rows are ignored

Validation overhead benchmark

Each pair uses identical Arrow arrays, storage conversion, and extension wrapping. The only added work in With is the production validator.

Case Without With Added validation Slowdown
MultiPoint 11.20 us 218.6 us 207.4 us 19.5x
MultiPolygon 62.54 us 246.4 us 183.9 us 3.94x
MultiPolygon, sparse nulls 57.54 us 224.3 us 166.8 us 3.90x
Point 0.268 us 313.7 us 313.4 us 1,171x
Point, sparse nulls 1.749 us 582.8 us 581.1 us 333x
Rect 0.421 us 496.5 us 496.1 us 1,178x

The high Point and Rect ratios compare a full buffer scan with a sub-microsecond zero-copy wrapper that never reads coordinate data. Over roughly one million coordinates or rows, the absolute validation cost is about 0.18-0.47 ns per coordinate/row, with 53-91 GB/s dense scan throughput. Sparse Point is slower because its regular null pattern creates about 105,000 short valid runs.

Signed-off-by: Nemo Yu <zyu379@wisc.edu>
@codspeed-hq

codspeed-hq Bot commented Aug 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 1842 untouched benchmarks
⏩ 44 skipped benchmarks1


Comparing nemo/geo-coordinate-validation (4fc3466) with develop (1525adf)

Open in CodSpeed

Footnotes

  1. 44 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Signed-off-by: Nemo Yu <zyu379@wisc.edu>
Signed-off-by: Nemo Yu <zyu379@wisc.edu>
@HarukiMoriarty

Copy link
Copy Markdown
Contributor Author

Temporary close.

@HarukiMoriarty

Copy link
Copy Markdown
Contributor Author

Let's bring this back and see if more reasonable.

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.18321% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.70%. Comparing base (490dfac) to head (4fc3466).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
vortex-geo/src/extension/validation.rs 95.18% 8 Missing ⚠️
vortex-geo/src/tests/linestring.rs 95.91% 2 Missing ⚠️
vortex-geo/src/extension/mod.rs 97.91% 1 Missing ⚠️
vortex-geo/src/extension/multilinestring.rs 0.00% 1 Missing ⚠️
vortex-geo/src/extension/multipoint.rs 0.00% 1 Missing ⚠️
vortex-geo/src/extension/multipolygon.rs 0.00% 1 Missing ⚠️
vortex-geo/src/extension/polygon.rs 0.00% 1 Missing ⚠️

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant